gh-131798: JIT optimizer: Support custom binary op and property frames #143735
Conversation
Fidget-Spinner
changed the title
Lib/test/test_capi/test_opt.py
Outdated
| uops = get_opnames(ex) | ||
|
|
||
| self.assertIn("_BINARY_OP_SUBSCR_INIT_CALL", uops) | ||
| # _POP_TOP_NOP is a sign the optimizer ran and didn't hit bottom. |
There was a problem hiding this comment.
| # _POP_TOP_NOP is a sign the optimizer ran and didn't hit bottom. | |
| # _POP_TOP_NOP is a sign the optimizer ran and didn't bail out. |
Not sure if it's just me but maybe this is a clearer phrasing?
There was a problem hiding this comment.
bottom is the actual term for contradiction used in the JIT optimizer. It's an obscure academic term, so let me change it to contradiction.
| new_frame = PyJitRef_NULL; | ||
| ctx->done = true; | ||
| assert((this_instr + 2)->opcode == _PUSH_FRAME); | ||
| PyCodeObject *co = get_code_with_logging(this_instr + 2); |
There was a problem hiding this comment.
Might be good to add a comment about why are using +2, since I don't know if it's abundantly clear? IIUC, it's because _SAVE_RETURN_OFFSET is between _LOAD_ATTR_PROPERTY_FRAME and _PUSH_FRAME.
Lib/test/test_capi/test_opt.py
Outdated
| import dis | ||
| dis.dis(testfunc, adaptive=True) |
There was a problem hiding this comment.
| import dis | |
| dis.dis(testfunc, adaptive=True) |
Leftover from debugging?
There was a problem hiding this comment.
Woops yes, thanks!
| break; | ||
| } | ||
| _Py_UOpsAbstractFrame *f = frame_new(ctx, co, 0, NULL, 0); | ||
| f->locals[0] = owner; |
There was a problem hiding this comment.
I think we need to check if frame is NULL before this, right?
There was a problem hiding this comment.
This was an excellent catch, thank you. We could've let in a segfault because of this!
|
When you're done making the requested changes, leave the comment: |
2 participants
BINARY_OP_SUBSCR_INIT_FRAMEis the opcode for subscripting custom__getitem__.LOAD_ATTR_PROPERTYis the opcode for loading an attribute decorated with@property.I see a solid 0.5-1% speedup from this PR alone on Sam's fastmark (pyperformance subset).